PR Review

Good approach following the error wrapping pattern from the linked blog post. The DecodeFieldError / ReadFileError types with proper source() chaining will meaningfully improve debuggability.

Issues

P1 — Incomplete wrapping in legacy decode path

The structural path wraps child field errors for Struct, List, FixedSizeList, and Map. But in the legacy path, only Struct children (line ~990) get wrapped. The legacy create_list_scheduler (line 634) calls create_legacy_field_scheduler for the list's child without wrapping:

let items_scheduler =
    self.create_legacy_field_scheduler(&list_field.children[0], column_infos, buffers)?;

This should also wrap with DecodeFieldError for consistency. Same for any other legacy compound types (FixedSizeList, etc.) that recurse into children.

P1 — io_source loses original error category

Both From<DecodeFieldError> and From<ReadFileError> convert via Error::io_source(). This means if the original error was e.g. Error::invalid_input(...), the wrapped error becomes an IO variant. This changes the error category, which could break callers that match on error variants for control flow. Consider preserving the original error's category or using a category-agnostic wrapping mechanism.

Nit

The #[cfg(test)] block had a comment // test coalesce indices to ranges removed — this looks like an accidental drive-by deletion (the comment described the test below it).

Thanks for the thorough review! All three issues have been addressed in the latest commit:

1. P1 — Legacy decode path: Added DecodeFieldError wrapping in create_list_scheduler for child field errors, matching the structural path's behavior.

2. P1 — Error category preservation: Changed From<DecodeFieldError> and From<ReadFileError> to use Self::wrapped() instead of Self::io_source(), so the original error category is preserved in the chain.

3. Nit — Restored comment: Put back the // test coalesce indices to ranges comment above mod tests.

All 428 tests pass (362 lance-encoding + 66 lance-file).

Rebased onto current main (the branch had drifted behind). The only conflicts were in the decoder.rs / reader.rs test modules where main had since added neighbouring tests — kept both sets. The production changes (the DecodeFieldError / ReadFileError wrappers and the map_err sites) applied cleanly.

Re-verified locally: cargo test -p lance-encoding -p lance-file (the 6 wrapper tests pass), cargo clippy -p lance-encoding -p lance-file --tests, and cargo fmt are all clean. Ready for another look when you have a moment.